osu#2 - #81

osu.Game/Online/Multiplayer/MultiplayerRoomUserRole.cs (1)

6-10: Consider assigning explicit integer values for serialization stability.

This enum is used in MultiplayerRoomUser, which is serialized via MessagePack. Without explicit values, future insertions or reordering of members could break backward compatibility with existing serialized data.

♻️ Suggested improvement

     public enum MultiplayerRoomUserRole
     {
-        Player,
-        Referee,
+        Player = 0,
+        Referee = 1,
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game/Online/Multiplayer/MultiplayerRoomUserRole.cs` around lines 6 - 10,
Assign explicit integer values to the MultiplayerRoomUserRole enum to ensure
MessagePack serialization stability: update the MultiplayerRoomUserRole
definition (used by MultiplayerRoomUser) so each member has a fixed integer
(e.g., Player = 0, Referee = 1) to prevent future reordering/insertions from
breaking compatibility.

osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs (1)

189-189: Consider verifying actual bookmark values, not just count.

The assertion only checks that there are 2 bookmarks but doesn't verify they contain the expected values (500 and 1000). While the count check provides basic coverage, verifying actual values would make the test more robust.

💡 Optional: More thorough assertion

-            AddAssert("created difficulty has bookmarks", () => EditorBeatmap.Bookmarks.Count == 2);
+            AddAssert("created difficulty has bookmarks", () => EditorBeatmap.Bookmarks.SequenceEqual(new[] { 500, 1000 }));

Note: Similar improvements could be applied to assertions at lines 259, 320, and 406.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Tests/Visual/Editing/TestSceneEditorBeatmapCreation.cs` at line 189,
The test currently only checks the bookmark count; update the assertion that
uses AddAssert("created difficulty has bookmarks", () =>
EditorBeatmap.Bookmarks.Count == 2) to verify the actual bookmark values (e.g.,
assert EditorBeatmap.Bookmarks.SequenceEqual(new[] {500, 1000}) or check both
Contains(500) and Contains(1000) and order if relevant) so the test validates
contents not just count; apply same pattern to the other similar assertions
referenced (lines with AddAssert checks for bookmarks at the other test points).

osu.Game.Rulesets.Osu.Tests/Mods/TestSceneOsuModEasy.cs (2)

51-54: Consider catching the specific expected exception type.

The test expects ApplyToPlayer to throw InvalidOperationException on reapplication (per ModEasyWithExtraLives implementation). Catching the specific exception type would make the test's intent clearer and avoid masking unexpected exceptions.

♻️ Suggested narrower catch

-                        catch
+                        catch (System.InvalidOperationException)
                         {
-                            // don't care if this fails. in fact a failure here is probably better than the alternative.
+                            // Expected: ApplyToPlayer throws when applied to a player twice.
                         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Rulesets.Osu.Tests/Mods/TestSceneOsuModEasy.cs` around lines 51 -
54, The empty catch should be narrowed to the expected exception type so
unexpected errors aren't swallowed: change the broad catch in
TestSceneOsuModEasy around the ApplyToPlayer call (testing ModEasyWithExtraLives
reapplication) to catch InvalidOperationException specifically and handle it the
same way (i.e., ignore or assert as intended), leaving other exceptions to
surface.

---

41-41: Consider adding a null check for Player in PassCondition.

PassCondition may be evaluated before Player is initialized. While the test framework likely handles this, a defensive null check would make the test more robust and clarify intent.

🛡️ Suggested defensive null check

                 PassCondition = () =>
                 {
+                    if (Player == null)
+                        return false;
+
                     if (((ModEasyTestPlayer)Player).FailuresSuppressed > 0 && !reapplied)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Rulesets.Osu.Tests/Mods/TestSceneOsuModEasy.cs` at line 41,
PassCondition currently dereferences Player and casts it to ModEasyTestPlayer;
add a defensive null check to ensure Player is not null before accessing
((ModEasyTestPlayer)Player).FailuresSuppressed and before using reapplied logic.
Update the PassCondition implementation to first check Player != null (or use
pattern matching like "Player is ModEasyTestPlayer pe && pe.FailuresSuppressed >
0") so the condition short-circuits when Player is null and avoids a
NullReferenceException while preserving the existing behavior when a
ModEasyTestPlayer is present.

osu.Game/Overlays/Settings/Sections/InputSubsection.cs (2)

121-128: Consider using VERTICAL_PADDING for consistent spacing.

The ToggleableHeader uses a hardcoded Vertical = 12 margin while the base CreateHeader() uses VERTICAL_PADDING (which equals 11.5f). This creates a minor visual inconsistency between toggleable and regular subsection headers.

♻️ Suggested fix

 headerText = new OsuSpriteText
 {
     Text = InputSettingsStrings.Device(text),
     Font = OsuFont.Style.Heading2,
-    Margin = new MarginPadding { Vertical = 12 },
+    Margin = new MarginPadding { Vertical = VERTICAL_PADDING },
     X = 18,
     Y = -1,
 },

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game/Overlays/Settings/Sections/InputSubsection.cs` around lines 121 -
128, The ToggleableHeader creation uses a hardcoded MarginPadding Vertical = 12
which causes a visual mismatch with the base CreateHeader() that uses the
VERTICAL_PADDING constant (11.5f); update the ToggleableHeader's headerText
Margin to use VERTICAL_PADDING instead of 12 so both ToggleableHeader and
CreateHeader() share the same vertical spacing (locate the headerText
instantiation in InputSubsection.cs and replace the hardcoded Vertical value
with VERTICAL_PADDING).

---

133-147: The bidirectional binding logic is complex but correct.

The pattern works: the Disabled toggle on lines 141-143 prevents feedback loops when Current updates switchButton.Current. However, a simpler approach using a direct BindTarget on load might reduce complexity.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game/Overlays/Settings/Sections/InputSubsection.cs` around lines 133 -
147, Replace the manual bidirectional wiring in LoadComplete with a direct bind
target to simplify the logic: instead of subscribing to
switchButton.Current.ValueChanged and using Current.BindValueChanged with
temporary switchButton.Current.Disabled toggles, call
Current.BindTarget(switchButton.Current) (or
switchButton.Current.BindTarget(Current)) on load to connect the two bindables,
and keep only a simple one-way update for the Disabled state controlled by the
existing toggleable flag (e.g. set switchButton.Current.Disabled = !toggleable
when toggleable changes). This eliminates the temporary disabling dance in
LoadComplete and uses the built‑in BindTarget mechanism to avoid feedback loops
while preserving the toggleable behavior.

osu.Game.Tests/Visual/UserInterface/TestSceneLoadingLayer.cs (1)

138-138: Make PressableButton private. This internal test class is only used within TestSceneLoadingLayer and does not need public visibility. Making it private sealed reduces surface area with no downsides.

♻️ Proposed refactor

-        public partial class PressableButton : RoundedButton, IKeyBindingHandler<GlobalAction>
+        private sealed partial class PressableButton : RoundedButton, IKeyBindingHandler<GlobalAction>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Tests/Visual/UserInterface/TestSceneLoadingLayer.cs` at line 138,
The nested test class PressableButton is unnecessarily public; change its
declaration inside TestSceneLoadingLayer from "public partial class
PressableButton" to "private sealed partial class PressableButton" (or "private
sealed class PressableButton" if partial isn't needed) to restrict visibility
and seal it, keeping the class usage the same but reducing the surface area.

osu.Game/Overlays/Toolbar/ToolbarButton.cs (1)

160-169: APIs verified. The HotkeyDisplay and Hotkey types exist with the expected signatures:

• HotkeyDisplay is a public class with a Hotkey property of type Hotkey
• Hotkey is a record struct with a constructor accepting GlobalAction

The code Hotkey = new Hotkey(Hotkey.Value) correctly passes the non-null GlobalAction value to the constructor. While the naming does involve Hotkey appearing three times in different contexts (ToolbarButton property, HotkeyDisplay property, and struct type), this is standard C# scoping and is contextually clear in the object initializer. If readability is a concern, an intermediate local variable could improve clarity, but it is not required.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game/Overlays/Toolbar/ToolbarButton.cs` around lines 160 - 169, The code
is correct but the repeated identifier "Hotkey" (the ToolbarButton property, the
HotkeyDisplay.Hotkey property, and the Hotkey record struct) is confusing; to
clarify, capture the ToolbarButton's Nullable<GlobalAction> into a local (e.g.
var globalAction = Hotkey.Value) inside the if (Hotkey != null) block and then
set HotkeyDisplay.Hotkey using that local (Hotkey = new Hotkey(globalAction)),
leaving all other behavior unchanged and keeping references to HotkeyDisplay and
the Hotkey record type.

osu.Game/Screens/OnlinePlay/Matchmaking/Queue/QueueController.cs (1)

193-194: invitation parameter is currently unused in Complete(...).

Either use invitation.Type for pool-specific UX or explicitly discard it to signal intent.

♻️ Minimal clarity tweak

-            public void Complete(MatchmakingRoomInvitationParams invitation)
+            public void Complete(MatchmakingRoomInvitationParams _)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game/Screens/OnlinePlay/Matchmaking/Queue/QueueController.cs` around
lines 193 - 194, The Complete(MatchmakingRoomInvitationParams invitation) method
currently ignores its invitation parameter; either consume invitation.Type where
room-pool-specific behavior or explicitly discard the parameter to document
intent. Update the Complete method (in QueueController.cs) to reference
invitation.Type in the UX/path-selection logic if pool-specific handling is
required, otherwise add an explicit discard (or comment) for the invitation
parameter so static analysis and readers know the unused parameter is
intentional.

osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderReversal.cs (1)

293-298: Assertion names don't match the indices used.

The indices and assertion messages appear swapped:

• ControlPoints[^2] is the second-to-last point (in the last arc), but the message says "First arc's control"
• ControlPoints[1] is the second point (in the first arc), but the message says "Last arc's control"

This is confusing for future maintainers. Consider swapping the assertion names to match the actual indices being checked.

Proposed fix: Swap assertion names

-AddAssert("First arc's control is not at the slider's middle", () =>
+AddAssert("Last arc's control is not at the slider's middle", () =>
     Vector2.Distance(selectedSlider.Path.ControlPoints[^2].Position, selectedSlider.Path.PositionAt(0.5)) > 1
 );

-AddAssert("Last arc's control is not at the slider's middle", () =>
+AddAssert("First arc's control is not at the slider's middle", () =>
     Vector2.Distance(selectedSlider.Path.ControlPoints[1].Position, selectedSlider.Path.PositionAt(0.5)) > 1
 );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderReversal.cs` around lines
293 - 298, The two AddAssert calls have swapped assertion messages vs the
control-point indices; update the descriptive strings so they match the indices
being checked — when asserting
Vector2.Distance(selectedSlider.Path.ControlPoints[^2].Position,
selectedSlider.Path.PositionAt(0.5)) use a message stating "Last arc's control
is not at the slider's middle", and when asserting
Vector2.Distance(selectedSlider.Path.ControlPoints[1].Position,
selectedSlider.Path.PositionAt(0.5)) use "First arc's control is not at the
slider's middle" so the messages correspond to the actual ControlPoints indices
in TestSceneSliderReversal.

osu.Game.Tests/Visual/Multiplayer/TestSceneFreeModSelectOverlay.cs (2)

81-85: Scope SelectAllModsButton lookup to the overlay container.

Using this.ChildrenOfType<SelectAllModsButton>().Single() is brittle in the new screen model because both footer and overlay can host similarly-typed controls. Prefer querying from freeModSelectOverlay (or another precise parent) to avoid ambiguous Single() matches.

Suggested change

-InputManager.MoveMouseTo(this.ChildrenOfType<SelectAllModsButton>().Single());
+InputManager.MoveMouseTo(freeModSelectOverlay.ChildrenOfType<SelectAllModsButton>().Single());

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Tests/Visual/Multiplayer/TestSceneFreeModSelectOverlay.cs` around
lines 81 - 85, The test currently locates the SelectAllModsButton globally via
this.ChildrenOfType<SelectAllModsButton>().Single(), which can return ambiguous
matches; change the lookup to scope to the overlay container (e.g. use
freeModSelectOverlay.ChildrenOfType<SelectAllModsButton>().Single() or otherwise
query from freeModSelectOverlay) in the AddStep that moves and clicks the button
so the test targets the overlay's button explicitly (keep the existing
InputManager.MoveMouseTo and InputManager.Click usage but obtain the target from
freeModSelectOverlay instead of this).

---

242-246: Dispose overlay registration before calling base.Dispose().

Disposing registration first gives deterministic teardown ordering for overlay lifecycle and avoids expiring overlay state after parent disposal.

Suggested change

 protected override void Dispose(bool isDisposing)
 {
-    base.Dispose(isDisposing);
     overlayRegistration?.Dispose();
+    base.Dispose(isDisposing);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Tests/Visual/Multiplayer/TestSceneFreeModSelectOverlay.cs` around
lines 242 - 246, Move the overlay registration disposal to occur before the base
class teardown: in the override Dispose(bool isDisposing) of
TestSceneFreeModSelectOverlay, call overlayRegistration?.Dispose() before
calling base.Dispose(isDisposing) so the overlay registration is torn down
deterministically prior to parent disposal and avoids expired overlay state
after the base class disposes.

osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.KeyButton.cs (1)

145-156: Logic is sound; the edge case of modifier-only triggers is unlikely but theoretically possible.

The conditional virtual-key mapping for global vs gameplay bindings is correctly implemented. However, the method's documentation states "only allowing a single non-modifier key to be specified", yet the code doesn't enforce this. If triggerKey is a modifier key (e.g., a user binds only Shift), it could appear twice in the keys array: once from filtering fullState.Keys and once from appending. Whether this causes issues depends on KeyCombination's internal handling, which cannot be verified without the framework code.

In normal usage, this is unlikely to occur, as users typically bind non-modifier keys. If framework conventions include deduplication, no action is needed.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game/Overlays/Settings/Sections/Input/KeyBindingRow.KeyButton.cs` around
lines 145 - 156, The current assembly of the keys array can produce duplicate
modifier entries or multiple non-modifier keys (contradicting the method doc);
modify the logic around fullState.Keys, triggerKey and combination creation so
you (1) deduplicate identical keys (e.g. via a distinct step) to prevent a
modifier appearing twice, and (2) enforce "only a single non-modifier key" by
ensuring at most one non-modifier remains (if multiple non-modifiers are
present, drop all but the intended triggerKey or the last non-modifier) before
calling UpdateKeyCombination(new KeyCombination(...)); operate on the variables
shown (fullState.Keys, triggerKey, keys, KeyBinding.Value.RulesetName,
GetVirtualKey, KeyCombination) so the global-vs-gameplay virtual-key mapping
logic is preserved.

osu.Game/Tests/Visual/ScreenTestScene.cs (1)

56-56: Resolve the TODO before merge (or track it explicitly).

Line [56] leaves uncertainty in a core test-scene composition path. Please either remove the TODO with a decision, or convert it into a tracked issue.

If you want, I can draft the follow-up issue text with acceptance criteria.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game/Tests/Visual/ScreenTestScene.cs` at line 56, Remove or resolve the
stray TODO comment inside the ScreenTestScene composition path: either delete
the line or replace it with a reference to a tracked issue ID and brief
rationale; update the ScreenTestScene class (where the TODO sits) to reflect the
decision (deleted comment if resolved or a comment like "See ISSUE-1234: reason"
if tracked) so the ambiguity is removed before merging.

osu.Game/Online/API/Requests/Responses/APITeam.cs (1)

21-21: Minor observation: Nullable type with non-null default.

Making FlagUrl nullable is appropriate since the API may not always provide a flag URL. The non-null default (string.Empty) provides a safe fallback, though it's slightly unconventional for nullable types. This approach works but consider if null would be a more semantically correct default for a nullable property.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game/Online/API/Requests/Responses/APITeam.cs` at line 21, The FlagUrl
field is declared nullable but initialized to an empty string; change this to be
semantically consistent by either making FlagUrl non-nullable (remove the '?')
and keep the empty-string default, or keep it nullable and set the default to
null; update the APITeam class's FlagUrl declaration accordingly (refer to the
FlagUrl field in APITeam) so the type and default value match the intended
semantics.

osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectFiltering.cs (3)

575-589: Consider using SingleOrDefault with explicit null check for robustness.

The Single(d => d.Enabled.Value) call will throw InvalidOperationException if zero or multiple spread displays are enabled, which could lead to confusing test failures. Consider using SingleOrDefault with an explicit assertion for clearer diagnostics.

🛡️ Proposed defensive approach

     private void scopeBeatmap(bool grouped)
     {
         if (grouped)
         {
             AddUntilStep("wait for spread indicator", () => this.ChildrenOfType<PanelBeatmapSet.SpreadDisplay>().Any(d => d.Enabled.Value));
-            AddStep("click spread indicator", () => this.ChildrenOfType<PanelBeatmapSet.SpreadDisplay>().Single(d => d.Enabled.Value).TriggerClick());
+            AddStep("click spread indicator", () =>
+            {
+                var display = this.ChildrenOfType<PanelBeatmapSet.SpreadDisplay>().SingleOrDefault(d => d.Enabled.Value);
+                Assert.That(display, Is.Not.Null, "Expected exactly one enabled spread display");
+                display!.TriggerClick();
+            });
         }
         else
         {
             AddUntilStep("wait for spread indicator", () => this.ChildrenOfType<PanelBeatmapStandalone.SpreadDisplay>().Any(d => d.Enabled.Value));
-            AddStep("click spread indicator", () => this.ChildrenOfType<PanelBeatmapStandalone.SpreadDisplay>().Single(d => d.Enabled.Value).TriggerClick());
+            AddStep("click spread indicator", () =>
+            {
+                var display = this.ChildrenOfType<PanelBeatmapStandalone.SpreadDisplay>().SingleOrDefault(d => d.Enabled.Value);
+                Assert.That(display, Is.Not.Null, "Expected exactly one enabled spread display");
+                display!.TriggerClick();
+            });
         }
 
         WaitForFiltering();
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectFiltering.cs` around
lines 575 - 589, In scopeBeatmap, avoid calling Single(...) on the enabled
SpreadDisplay collections; replace the Single( d => d.Enabled.Value ) usages for
both PanelBeatmapSet.SpreadDisplay and PanelBeatmapStandalone.SpreadDisplay with
SingleOrDefault(...) and then add an explicit assertion/guard that the result is
not null (and optionally that no duplicate enabled items exist) before calling
TriggerClick(), so test failures report a clear assertion instead of an
InvalidOperationException.

---

591-591: Helper findBeatmap could benefit from defensive handling.

First() will throw if no beatmap matches the substring, which could result in unclear test failures. Consider adding a fallback or clearer error message.

🛡️ Proposed defensive approach

-    private BeatmapInfo findBeatmap(string difficultySubstring) => Beatmap.Value.BeatmapSetInfo.Beatmaps.First(b => b.DifficultyName.Contains(difficultySubstring));
+    private BeatmapInfo findBeatmap(string difficultySubstring)
+    {
+        var beatmap = Beatmap.Value.BeatmapSetInfo.Beatmaps.FirstOrDefault(b => b.DifficultyName.Contains(difficultySubstring));
+        Assert.That(beatmap, Is.Not.Null, $"Could not find beatmap with difficulty name containing '{difficultySubstring}'");
+        return beatmap!;
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectFiltering.cs` at line
591, The helper findBeatmap currently uses First(...) which throws a generic
exception when no match is found; update findBeatmap to use FirstOrDefault(...)
on Beatmap.Value.BeatmapSetInfo.Beatmaps, then check for null and throw a clear
exception (or call Assert.Fail) that includes the searched difficultySubstring
and the available beatmap difficulty names to make test failures actionable;
reference the findBeatmap method and the Beatmap.Value.BeatmapSetInfo.Beatmaps
sequence when making this change.

---

420-451: Unnecessary cleanup step on line 450.

The TestUnscopeRevertsToOriginalSelection test doesn't set any star difficulty filter, yet line 450 resets DisplayStarsMaximum to 10.1. This cleanup step appears to be copy-pasted from another test and is unnecessary here.

🧹 Proposed fix to remove unnecessary cleanup

         checkMatchedBeatmaps(6);
         AddAssert("normal difficulty is selected", () => Beatmap.Value.BeatmapInfo, () => Is.EqualTo(findBeatmap("Normal")));
-
-        AddStep("reset star difficulty filter", () => Config.SetValue(OsuSetting.DisplayStarsMaximum, 10.1));
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelectFiltering.cs` around
lines 420 - 451, The test method TestUnscopeRevertsToOriginalSelection contains
an unnecessary cleanup AddStep that calls
Config.SetValue(OsuSetting.DisplayStarsMaximum, 10.1); remove that AddStep (the
step resetting the star difficulty filter) from the method so the test no longer
performs the irrelevant Config.SetValue call; locate the call by searching for
Config.SetValue and OsuSetting.DisplayStarsMaximum within
TestUnscopeRevertsToOriginalSelection and delete that single AddStep block.

osu.Game.Tests/Visual/UserInterface/TestSceneFirstRunSetupOverlay.cs (2)

56-68: Reset lastNotification in setup to avoid cross-test state bleed.

lastNotification can carry stale data between tests if no new notification is posted.

♻️ Proposed change

             AddStep("setup dependencies", () =>
             {
                 performer.Reset();
                 notificationOverlay.Reset();
+                lastNotification = null;

                 performer.Setup(g => g.PerformFromScreen(It.IsAny<Action<IScreen>>(), It.IsAny<IEnumerable<Type>>()))
                          .Callback((Action<IScreen> action, IEnumerable<Type> _) => action(null));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Tests/Visual/UserInterface/TestSceneFirstRunSetupOverlay.cs` around
lines 56 - 68, Reset lastNotification at the start of the "setup dependencies"
step to avoid cross-test state bleed: inside the AddStep lambda that currently
calls performer.Reset() and notificationOverlay.Reset(), set lastNotification =
null (or appropriate default) before configuring performer.Setup(...) and
notificationOverlay.Setup(...); reference symbols: lastNotification,
AddStep("setup dependencies", ...), performer.Reset, notificationOverlay.Reset,
performer.Setup, notificationOverlay.Setup.

---

218-218: Narrow Overlay mutability in the nested screen.

A property with a private setter prevents accidental reassignment from tests/helpers.

♻️ Proposed change

-            public FirstRunSetupOverlay Overlay = null!;
+            public FirstRunSetupOverlay Overlay { get; private set; } = null!;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Tests/Visual/UserInterface/TestSceneFirstRunSetupOverlay.cs` at line
218, Replace the publicly mutable field "Overlay" (type FirstRunSetupOverlay)
with a property that has a private setter to prevent external reassignment;
specifically change the declaration of Overlay from a public field to a public
property with a private set (keeping the null-forgiving initializer) and update
any assignments inside the nested screen to set the property internally (leave
external reads unchanged). Ensure references to "Overlay" elsewhere still
compile and that only the nested screen's code performs the assignment.

osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchFreestyleSelect.cs (1)

59-60: Update stale log source name in abort message.

On Line 59, the message uses nameof(OnStart) inside StartAction(), which makes diagnostics misleading.

♻️ Proposed fix

-                Logger.Log($"{nameof(OnStart)} aborted due to {nameof(operationInProgress)}");
+                Logger.Log($"{nameof(StartAction)} aborted due to {nameof(operationInProgress)}");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchFreestyleSelect.cs`
around lines 59 - 60, The log message inside StartAction() uses the stale
nameof(OnStart) which is misleading; update the Logger.Log call in StartAction
(the line currently logging $"{nameof(OnStart)} aborted due to
{nameof(operationInProgress)}") to reference nameof(StartAction) instead (e.g.
$"{nameof(StartAction)} aborted due to {nameof(operationInProgress)}") so
diagnostics reflect the correct source.

osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs (1)

252-256: Strengthen the abort test to avoid false positives.

At Line 252 and Line 255, the test aborts after Key.Down without proving the selection actually changed first. If focus/input fails, this can still pass. Add an intermediate assertion that the freestyle selection moved before aborting.

Proposed test hardening

             AddStep("select next beatmap", () => InputManager.Key(Key.Down));
+            AddUntilStep("selection changed in freestyle select",
+                () => !Beatmap.Value.BeatmapInfo.Equals(importedSet.Beatmaps[0]));
             AddStep("abort", () => playlistsScreen.Stack.CurrentScreen.Exit());
 
             AddUntilStep("beatmap not changed", () => Beatmap.Value.BeatmapInfo.Equals(importedSet.Beatmaps[0]));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs` around
lines 252 - 256, The test currently presses Key.Down then aborts without
verifying the selection actually moved; modify the sequence around
AddStep("select next beatmap", () => InputManager.Key(Key.Down)) and before
calling playlistsScreen.Stack.CurrentScreen.Exit() to first capture the
pre-selection (Beatmap.Value.BeatmapInfo), then assert the selection changed
(e.g. via AddUntilStep or AddAssert that Beatmap.Value.BeatmapInfo != previous
or equals the expected next beatmap from importedSet.Beatmaps[1]) to ensure the
Key.Down had effect before aborting; reference symbols: AddStep,
InputManager.Key(Key.Down), Beatmap.Value.BeatmapInfo, importedSet.Beatmaps,
AddUntilStep/AddAssert, playlistsScreen.Stack.CurrentScreen.Exit().